-
Notifications
You must be signed in to change notification settings - Fork 144
Add reduce function to arrays #4290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
devin-ai-integration
wants to merge
11
commits into
master
Choose a base branch
from
devin/1760470003-add-array-reduce-function
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add reduce function to arrays #4290
devin-ai-integration
wants to merge
11
commits into
master
from
devin/1760470003-add-array-reduce-function
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implement array reduce function with signature: fun reduce<U>(initial: U, _ f: fun (U, T): U): U Changes: - sema: Define ArrayReduceFunctionType and register in getArrayMembers - interpreter: Implement ArrayValue.Reduce and NativeArrayReduceFunction - vm: Register reduce function for both variable and constant-sized arrays - tests: Add tests for reduce in arrays_dictionaries_test.go and misc_test.go
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Benchstat comparison
Results
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
turbolent
reviewed
Oct 15, 2025
Per review comment: add function signature comments similar to ArrayMapFunctionType
|
Added comments in commit 86d3e8e. The function now includes:
Similar to how |
turbolent
reviewed
Oct 15, 2025
Per review comment: inline the argumentTypes slice directly in the invokeFunctionValue call
|
Done in commit a8942f8. Inlined |
turbolent
reviewed
Oct 15, 2025
function type is the same for variable-sized and constant-sized array types
Per review comment: Keep only the function signature comment
…git-manager.devin.ai/proxy/github.com/onflow/cadence into devin/1760470003-add-array-reduce-function
turbolent
reviewed
Oct 15, 2025
turbolent
approved these changes
Oct 15, 2025
…git-manager.devin.ai/proxy/github.com/onflow/cadence into devin/1760470003-add-array-reduce-function
18 tasks
RZhang05
reviewed
Oct 15, 2025
…based inner array mutation; update expectations
RZhang05
reviewed
Oct 15, 2025
RZhang05
reviewed
Oct 15, 2025
RZhang05
approved these changes
Oct 15, 2025
SupunS
reviewed
Oct 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds a
reducefunction to arrays in Cadence with the signature:The implementation follows the same three-layer pattern as the existing
mapfunction:Ufor the accumulator type, allowing it to differ from the array element typeTChanges
Type System
ArrayReduceFunctionTypewith generic type parameterUInterpreter
ArrayValue.Reducemethod that iterates and accumulates valuesNativeArrayReduceFunctionwrapperGetMethodfor array valuesTests
TestInterpretArrayReducefrom misc_test.go to array_test.go (per review feedback)ContainerMutatedDuringIterationError)Critical Review Areas
Generic type parameter handling: Verify
ArrayReduceFunctionTypecorrectly handles the case where accumulator typeUdiffers from element typeTStruct array test: The test
testStructArrayReduceoperates on an unauthorized reference&[S]and mutates struct fields inside the reducer. This works because struct elements are passed by value (copies), not by reference. Verify this is the intended test case - it demonstrates that mutations happen on copies when using unauthorized references.Resource array rejection: Confirm that rejecting reduce on resource arrays matches the intended behavior and is consistent with map
Link to Devin run: https://app.devin.ai/sessions/f6bc7843ea324388964f82a1cdc1b1b1
Requested by: [email protected]
masterbranchFiles changedin the Github PR explorer